Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solution #748

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Solution #748

wants to merge 7 commits into from

Conversation

sind14
Copy link

@sind14 sind14 commented Jan 8, 2025

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your hard work on this project! There are a few critical issues that need to be addressed before we can approve the pull request. Specifically, the TicketViewSet is missing a serializer_class attribute, which is essential for defining the serializer to use. Additionally, there's a logical error in the condition if self.action == ("list", "retrieve"): that needs to be corrected to if self.action in ["list", "retrieve"]:. Lastly, ensure that the data.get method uses parentheses correctly. Once these issues are resolved, your code will be in great shape! Keep up the great work! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

Comment on lines 108 to 110
movie_session=data.get["movie_session"],
row=data.get["row"],
seat=data.get["seat"],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data.get method should use parentheses instead of square brackets. Correct it to data.get('movie_session'), data.get('row'), and data.get('seat').

cinema/views.py Outdated
@@ -43,6 +57,28 @@ def get_serializer_class(self):

return MovieSerializer

def get_queryset(self):
queryset = super().get_queryset()
if self.action == ("list", "retrieve"):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition if self.action == ("list", "retrieve"): is incorrect. It should be if self.action in ["list", "retrieve"]: to properly check if the action is either 'list' or 'retrieve'.

serializer.save(user=self.request.user)

class TicketViewSet(viewsets.ModelViewSet):
queryset = Ticket.objects.all()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TicketViewSet is missing a serializer_class attribute. You should define it to specify which serializer to use for this viewset.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job on addressing the issues from the previous review! 🎉 Your code now meets the task requirements and constraints, and is ready for approval. Here are some key points from the review:

  • The OrdersPagination class and serializers in serializers.py are well-structured and align with Django REST Framework conventions. The use of nested serializers and related fields is appropriate, and the OrderSerializer ensures atomicity when creating orders and tickets.

  • The urls.py file correctly sets up URL routing using Django REST Framework's DefaultRouter, and the views.py file contains well-implemented viewsets for handling CRUD operations. The MovieViewSet and MovieSessionViewSet have custom logic to handle different actions and query parameters.

  • The settings.py file is well-configured for development, but remember to turn off DEBUG and secure the SECRET_KEY for production environments.

  • You've made important corrections, such as including the serializer_class in TicketViewSet, fixing the logical error in conditions, and using the data.get method correctly.

While the code is approved, please ensure to address any critical issues in the future to avoid similar problems. Keep up the great work and continue learning from these experiences! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

Anton Hryhorenko added 2 commits January 10, 2025 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants